feat(claude-ops): add inventory skill enumerating the invocable surface - #2313

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

Claude finished @kyle-sexton's task in 1m 25s —— View job

Security Review — PR #2313

• Gather context (PR diff, changed files)
• Read changed files (inventory.py, test_inventory.py, SKILL.md, extraction.md, evals.json, plugin.json, CHANGELOG.md, CATALOG.md, SKILL-CHEAT-SHEET.md)
• Analyze for security issues (injection, unsafe file/binary handling, path traversal, secrets)
• Post findings

Result: No security issues found

I reviewed only the files changed in this PR (git diff origin/main...HEAD), focused on plugins/claude-ops/skills/inventory/scripts/inventory.py (893 lines, the actual attack surface — it's the only new executable code) plus its test file and the three markdown docs.

What the script does, from a trust-boundary perspective:

• Reads the local claude binary and local JSON config files (settings.json, known_marketplaces.json, plugin manifests) — all already-local, already-trusted-or-not-newly-exposed data.
• Parses everything with re/json.load — no eval, exec, pickle, subprocess, or shell=True anywhere in the diff.
• Writes only to a caller-specified --out path (local CLI arg, not attacker-reachable).
• Never executes or modifies the binary it reads (confirmed: only .read_bytes() / .stat() calls on it).

Specific things I checked and ruled out:

• Path handling (config_dir(), --binary, --config-dir, scan_marketplace's use of installLocation from disk JSON): all local-CLI-controlled or read from config the user already owns; no elevation of privilege, no traversal into a new trust domain — worst case is reading/reporting on directories already reachable by the invoking user.
• Regex extraction functions (_TYPE_RE, _NAME_RE, extract_plugin_backed's bounded {0,900} lookahead loop, etc.): bounded quantifiers, no catastrophic-backtracking shape, and the PR's own test plan reports a 2.7s run against a 25MB real bundle — consistent with no ReDoS risk.
• _load_json: pure json.load, no unsafe deserialization; a malicious plugin.json's dependencies field is echoed verbatim into the JSON report, but that's inert data in a locally-written report, not a code-execution path.
• SKILL.md / reference/extraction.md: instructs the agent to run the script and to curl -sSL two hardcoded upstream URLs (code.claude.com/docs/..., raw.githubusercontent.com/anthropics/claude-code/...) for doc-drift verification — no user-controlled or externally-sourced URL construction, no command injection surface.
• No GitHub Actions files are touched by this PR — the workflow-trigger/permissions/supply-chain concerns in my review scope don't apply here.
• test_inventory.py: hermetic unit tests against synthetic strings, no I/O beyond importing the module.

This is a read-only, local-only diagnostic tool with no new injection, authorization, or secret-handling surface. Nothing to flag.

Claude finished @kyle-sexton's task in 4m 30s —— View job

Claude's review

Todo list

• Gather context (CLAUDE.md, REVIEW.md, diff)
• Review inventory.py (extraction logic, error handling)
• Review test_inventory.py (test coverage)
• Review SKILL.md and reference docs
• Review plugin manifest/changelog/catalog updates
• Post findings as inline comments + summary

Summary

Reviewed the changed files for correctness, error handling, test coverage, and maintainability (this repo has a claude-security-review workflow, so security findings are out of scope for this lane per REVIEW.md). Two 🔴 Important correctness/error-handling findings, posted inline:

1. scripts/inventory.py#L789-L797 — --self-check swallows its own diagnostic on the most common failure. When no claude executable is found at all, the "why" lands under the key "reason", but the --self-check reader in main() only ever looks up "error" (L870-872) — so a fresh machine with no claude on PATH prints the generic BROKEN: binary source unavailable instead of the specific reason pick_binary() already computed. This undercuts the PR's own stated design goal for this feature ("every run ... tells on itself").

2. scripts/inventory.py#L517-L544 — bundled-skill fields can bleed from the next registration. extract_bundled_skills reads aliases/gated/hidden/description from a fixed forward text window rather than bounding to the current xu({...}) call's own close, so a skill missing one of those optional fields inherits it from whichever skill is registered next. Traced through the test fixture: dataviz (no aliases of its own) would resolve aliases: ["checkup"] — doctor's aliases — because the search keeps scanning past dataviz's own object. This is exactly the failure mode build_brace_map was built to prevent for command objects (documented in reference/extraction.md as "the single most important correctness property in the script"), but that discipline wasn't extended to this path. Not caught by the existing suite, since test_constant_names_resolve only asserts on code-review's aliases, which happens to be populated on its own object.

Both come with a [Fix this →] link in the inline comment.

What I couldn't verify: Bash execution wasn't available in this review environment, so I could not run test_inventory.py myself; the PR's Test Plan claims (25 passing tests, live run against 2.1.228, etc.) are author-reported and unverified by me. Finding #2 is a logical trace through the code and the fixture in test_inventory.py, not an executed repro.

Otherwise: the extraction design (runtime registrar discovery, brace-depth field resolution for commands, integrity verdict with canaries/yield-ratio/registrar-sweep) is careful and well-documented; the manifest/changelog/catalog/cheat-sheet updates are mechanical and consistent with the new skill.

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

Claude finished @kyle-sexton's task in 1m 50s —— View job

Security Review — PR #2313

• Gather context (PR diff, changed files)
• Read changed files (inventory.py, test_inventory.py, SKILL.md, extraction.md, evals.json, plugin.json, CHANGELOG.md, CATALOG.md, SKILL-CHEAT-SHEET.md)
• Analyze for security issues (injection, unsafe file/binary handling, path traversal, secrets, GitHub Actions risk)
• Post findings

Result: No security issues found

Reviewed all 9 files changed in this PR (current HEAD 3fa2e33), with focus on scripts/inventory.py (893 lines — the only new executable code and the actual attack surface). No .github/workflows files are touched by this PR, so the GitHub Actions concerns in my review scope (trigger patterns, permissions, pull_request_target/workflow_run, script injection via the github context) don't apply here.

Trust-boundary analysis of inventory.py:

• No subprocess, os.system, eval, exec, Popen, or shell=True anywhere in the file — confirmed by direct grep, not just inspection.
• Every filesystem read is local, already-user-owned data: the shipped claude binary (.read_bytes()/.stat() only, never written or executed — confirmed at read_bundle), settings.json, known_marketplaces.json, and plugin manifests under the config dir, all parsed with json.load (_load_json, L660-L665) — no pickle, no unsafe deserialization.
• --out, --binary, and --config-dir are local CLI arguments controlled by the invoking user, at the same trust level as the process itself — writing to --out or reading from --config-dir/--binary is not a privilege boundary crossing, and there's no scenario in the skill's usage (SKILL.md) where these are populated from untrusted/remote input.
• installLocation from known_marketplaces.json (scan_disk) is data the claude CLI itself already wrote to local disk when the marketplace was added — used only as a Path for a read-only iterdir() scan, never interpolated into a command or path-joined with ..-style traversal risk beyond what the user already controls.
• Regex extraction (_TYPE_RE, extract_plugin_backed's bounded {0,900} lookahead, etc.) uses bounded quantifiers with no catastrophic-backtracking shape; the PR's reported 2.7s run against a 25MB real bundle is consistent with no ReDoS.
• manifest.get("dependencies") is echoed verbatim into the JSON report — inert data reflected into a locally-written file, not a code-execution or injection path.
• SKILL.md/reference/extraction.md reference curl -sSL only against two hardcoded upstream URLs (code.claude.com/docs/..., raw.githubusercontent.com/anthropics/claude-code/...) for doc-drift verification — no user- or externally-controlled URL construction.
• test_inventory.py is hermetic unit tests against synthetic in-memory strings — no I/O, no secrets.
• plugin.json, CHANGELOG.md, CATALOG.md, SKILL-CHEAT-SHEET.md changes are description/version/doc-index updates only — no permission, hook, or scope changes.

This is a read-only, local-only diagnostic tool with no new injection, authorization, or secret-handling surface. (Note: the correctness/logic findings already raised by Codex and the earlier Claude review pass in this thread — field-window bleed in extract_bundled_skills, --self-check's error-key mismatch, marketplace-vs-installed scanning — are legitimate but out of scope for this security-only lane.)